[V0 deprecation] Remove _is_v1_supported_oracle#25673
[V0 deprecation] Remove _is_v1_supported_oracle#25673MatthewBonanni wants to merge 2 commits intovllm-project:mainfrom
Conversation
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
Signed-off-by: Matthew Bonanni <mbonanni@redhat.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the _is_v1_supported_oracle function and its usage, which handled automatic fallbacks to the V0 engine. With this change, the V1 engine becomes the default unless explicitly disabled with VLLM_USE_V1=0. This aligns with the goal of deprecating V0. The changes are clean, and the corresponding test removal from the CI pipeline is appropriate. I find the implementation to be correct and have no further comments.
| # V1 supports N-gram, Medusa, and Eagle speculative decoding. | ||
| if self.speculative_config is not None: | ||
| # speculative_config could still be a dict at this point | ||
| if isinstance(self.speculative_config, dict): | ||
| method = self.speculative_config.get("method", None) | ||
| else: | ||
| method = self.speculative_config.method | ||
|
|
||
| if method == "draft_model": | ||
| raise NotImplementedError( | ||
| "Draft model speculative decoding is not supported yet. " | ||
| "Please consider using other speculative decoding methods " | ||
| "such as ngram, medusa, eagle, or deepseek_mtp.") |
There was a problem hiding this comment.
Hmmm, I think _is_v1_supported_oracle is still useful to exclude some unsupported arguments here
There was a problem hiding this comment.
Maybe we could make an _is_config_supported_oracle, which doesn't dictate VLLM_USE_V1, it just throws errors if unsupported.
Alternatively, it might be better if this check happens in the speculative initialization rather than at an engine level?
There was a problem hiding this comment.
Maybe we could make an _is_config_supported_oracle, which doesn't dictate VLLM_USE_V1, it just throws errors if unsupported.
Agree.
Alternatively, it might be better if this check happens in the speculative initialization rather than at an engine level?
In fact, these arguments are ever supported in v0 but haven't been implemented (some of them are deprecated) in v1. So I prefer to keep them here for clarity.
There was a problem hiding this comment.
+1, it's good raise NotImplementedError error instead of removing the check totally.
|
@MatthewBonanni @Isotr0py I post a new PR here #28116 since this PR is not updated for long time. Feel free to close mine if this PR is still alive. Thanks. |
|
@wangxiyuan Thanks! Sorry this PR slipped my mind. Closing. |
3 participants
Purpose
Remove
_is_v1_supported_oracleand corresponding testTest Plan
CI should suffice
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.